Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MNT/TST: Update SWAPI packet definition and tests #803

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

greglucas
Copy link
Collaborator

This removes some unnecessary utils functions and moves over to packet_file_to_datasets()

Note that updating the packet definition also changed the plan_id_science back to plan_id which was in the original spreadsheet definition. This is because they can now have different bit lengths between the packets.

The packet definition also does not have the AUT packet defined in it anymore, do we want to add that one to the definition, or leave it out? I left it out for now and just removed that test.

This removes some unnecessary utils functions and moves over
to packet_file_to_datasets()
@greglucas greglucas added Ins: SWAPI Related to the SWAPI instrument Level: L0 Level 0 processing labels Aug 30, 2024


@pytest.fixture(scope="session")
def decom_test_science_data(swapi_l0_test_data_path):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is interesting that they gave you "derived_values" for the science data, but didn't use any "derived_values" for the housekeeping. That is why we have these two different fixtures here now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. That's why I had to add if and else in test but your approach is much better.

@greglucas
Copy link
Collaborator Author

The failing tests are due to an issue upstream with space_packet_parser that has already been fixed, but not released yet. medley56/space_packet_parser#54

I'm not sure there is much we can do about this until the fix is out.

Copy link
Contributor

@tech3371 tech3371 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you again!



@pytest.fixture(scope="session")
def decom_test_science_data(swapi_l0_test_data_path):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. That's why I had to add if and else in test but your approach is much better.

@greglucas greglucas marked this pull request as draft September 19, 2024 20:03
@greglucas
Copy link
Collaborator Author

Converting to draft since this requires changes to Space Packet Parser that will be released in the next version. Need to revisit this then when we upgrade that dependency version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ins: SWAPI Related to the SWAPI instrument Level: L0 Level 0 processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants